ASoC: intel: bytcr_rt5651: add remove function to disable jack #1149
Conversation
|
a long shot following rt5682 fix... |
|
SOFCI TEST |
juimonen
force-pushed
the
rt5651_fix
branch
2 times, most recently
from
4622638
to
f37b0df
Compare
|
@juimonen you may want to work with Hans De Goede (@jwrdegoede), he's done quite a bit of rt5651 support for baytrail tablets and might have devices you can test this patch on. Though those devices are usually BYT-CR and we don't support them well with SOF so far due to this stupid SSP2-SSP0 swap. |
|
FYI, I've a list of all devices / codecs I can test on here:
https://github.com/jwrdegoede/sunxi-fedora-scripts/blob/master/x86-codec-info
Never mind the repo-name, that is a leftover from when I was working on Allwinner based tablets instead of on x86 tablets.
The VIOS LTH17 listed there is a Cherry Trail device with rt5651. I can run some tests if necessary, but I've been sorta swamped with work lately, so expect a slow turnaround time on testing requests and please spell out what exactly you want tested (microphone, or speakers or ... what behavior to check for / expect).
|
|
@jwrdegoede ok thanks! I'll try still to do couple of more iterations internally, I'll then think what to do... |
When removing sof module the support_button_press function will oops because hp_jack pointer is not checked for NULL. So add a check to fix this. Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>
|
@plbossart in rt5651 the headset detection work seems to be cancelled when the module is removed. However if I add a null check in the support_button_press function we don't get oops in module removal anymore. Don't know if this is a correct fix though... @jwrdegoede any thoughts? |
| @@ -1770,6 +1770,9 @@ static int rt5651_detect_headset(struct snd_soc_component *component) | |||
|
|
|||
| static bool rt5651_support_button_press(struct rt5651_priv *rt5651) | |||
| { | |||
| if (!rt5651->hp_jack) | |||
| return false; | |||
|
|
|||
| /* Button press support only works with internal jack-detection */ | |||
| return (rt5651->hp_jack->status & SND_JACK_MICROPHONE) && | |||
| rt5651->gpiod_hp_det == NULL; | |||
There was a problem hiding this comment.
if you look at the caller, the hp_jack is set to NULL after calling this function:
static void rt5651_disable_jack_detect(struct snd_soc_component *component)
{
struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
disable_irq(rt5651->irq);
rt5651_cancel_work(rt5651);
if (rt5651_support_button_press(rt5651)) {
rt5651_disable_micbias1_ovcd_irq(component);
rt5651_disable_micbias1_for_ovcd(component);
snd_soc_jack_report(rt5651->hp_jack, 0, SND_JACK_BTN_0);
}
rt5651->hp_jack = NULL;
}so something is fishy here.
There was a problem hiding this comment.
@plbossart yes for sure. I guess I'm delaying something with the addition or someone is calling set_jack(NULL) twice. As I understand this will access NULL if you call it twice? Not sure of the semantics of set_jack, I guess you should be able to call it twice with NULL...
There was a problem hiding this comment.
If it is, I think this fix is correct.
There was a problem hiding this comment.
This is the commit that added this change: 3bb936f
There was a problem hiding this comment.
@ranjani yeah could be... I don't have the rt5651 device to test this, usually these devices don't reboot nicely after kernel oops, so it is really hard also with remote access device.
|
This PR was not tested by SOF CI, so has anyone actually tried the code on a platform with rt5651? |
|
@plbossart only @Jiangxinx tested on device with rt5651: #1059 (comment) |
ok, thanks for the pointer. |
When removing sof module the rt5651 jack handler will oops
if jack detection is not disabled. So add remove function,
which disables the jack detection.
Signed-off-by: Jaska Uimonen jaska.uimonen@intel.com